packages/web-platform/web-elements/src/elements/XMarkdown/XMarkdownAttributes.ts (1)

512-531: Use boostedQueueMicrotask for the overflow measurement.

This post-render branch does DOM reads on the main thread but bypasses the package batching helper. Switching the overflow check to boostedQueueMicrotask keeps it aligned with the rest of the reactive pipeline and avoids unnecessary layout thrash.

As per coding guidelines, "Use boostedQueueMicrotask for DOM reads/writes to avoid synchronous layout thrashing".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/web-platform/web-elements/src/elements/XMarkdown/XMarkdownAttributes.ts`
around lines 512 - 531, Replace the direct queueMicrotask usage with the package
batching helper boostedQueueMicrotask for the post-render overflow measurement:
when this.#textMaxline > 0 schedule the DOM read inside boostedQueueMicrotask
(use the same root, the same overflow calculation root.scrollHeight >
root.clientHeight + 1, and the same dispatch using this.#dom.dispatchEvent and
this.#overflowEmitted flag) so the overflow check aligns with the reactive
pipeline and avoids layout thrash; ensure error handling around the check is
preserved.

packages/web-platform/web-elements/tests/x-markdown.spec.ts (1)

154-161: Replace fixed sleeps with state-based waits.

These assertions are pinned to internal timing constants (#appendFlushDelay and the typewriter speed), so they’ll get flaky as soon as CI is slower or those defaults change. Prefer waitForFunction / expect.poll against paragraph count, cursor presence, or the _drawEnd / _animationComplete flags that the fixtures already expose.

Also applies to: 233-269

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/web-platform/web-elements/tests/x-markdown.spec.ts` around lines 154
- 161, Tests in x-markdown.spec.ts use fixed waits (page.waitForTimeout) around
incremental rendering which makes them flaky; replace those sleeps in the
'should batch append by newline boundary' test (and other similar blocks around
lines 233-269) with state-based waits: after appendContent(page, 'Line 2') poll
for the desired condition using expect.poll or page.waitForFunction to check
getShadowCount(page, 'p') reaches 2 or to observe the component flags like
_drawEnd or _animationComplete on the element, and remove the hardcoded
20ms/80ms waits so the test waits for actual state changes instead of timing
assumptions.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

packages/web-platform/web-elements/tests/x-markdown.spec.ts (2)

433-441: Timing-dependent test may be flaky.

The test uses waitForTimeout(20) and waitForTimeout(80) to verify batch append behavior by newline boundary. These fixed delays can cause flakiness across different CI environments or under load. Consider using waitForFunction with a condition check instead, or document why these specific timing values are required.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/web-platform/web-elements/tests/x-markdown.spec.ts` around lines 433
- 441, The test 'should batch append by newline boundary' is flaky due to fixed
sleeps (page.waitForTimeout(20/80)); replace those timing-based assertions with
a polling wait that checks the actual condition instead—use Playwright's
page.waitForFunction or an expect.poll-style retry to wait until
getShadowCount(page, 'p') equals the expected value after calling
appendContent(page, 'Line 2') (first assert it stays 1 within a short window,
then wait for it to become 2), referencing the existing helpers goto,
appendContent, and getShadowCount to perform the condition checks rather than
fixed timeouts.

---

510-530: Multiple timing-dependent waits in typewriter cursor test.

The test uses waitForTimeout(1500) and waitForTimeout(1000) to verify cursor rendering and removal. These long delays make the test slow and potentially flaky. Consider exposing a test-only signal or using waitForFunction to poll for cursor presence/absence instead.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/web-platform/web-elements/tests/x-markdown.spec.ts` around lines 510
- 530, Replace the two hard-coded waits in the test 'should render custom
typewriter cursor' with deterministic polling: after navigating with goto and
waiting for window._drawStart, use page.waitForFunction to wait until the
shadowRoot querySelector('#cursor') returns truthy (cursor present) and then use
another page.waitForFunction to wait until that same selector returns falsy
(cursor removed); remove the waitForTimeout(1500) and waitForTimeout(1000) calls
and keep the existing queries (document.querySelector('x-markdown')!,
el.shadowRoot, '#cursor') so the assertions remain the same but are driven by
waitForFunction instead of fixed delays.

packages/web-platform/web-elements/src/elements/XMarkdown/XMarkdown.ts (1)

16-91: Significant code duplication with XMarkdownAttributes.ts.

The helper functions getComposedRange, createRangeByOffsets, and isSelectionNodeInsideHost are duplicated between this file and XMarkdownAttributes.ts (lines 162-237). Consider extracting these into a shared utility module to reduce maintenance burden and ensure consistent behavior.

♻️ Suggested approach

Create a shared utility file like XMarkdownSelectionUtils.ts:

// XMarkdownSelectionUtils.ts
export const getComposedRange = (
  selection: Selection,
  shadowRoot: ShadowRoot | null,
): StaticRange | null => { /* ... */ };

export const createRangeByOffsets = (
  doc: Document,
  root: HTMLElement,
  start: number,
  end: number,
): Range | null => { /* ... */ };

export const isSelectionNodeInsideHost = (
  dom: HTMLElement,
  shadowRoot: ShadowRoot | null,
  node: Node | null,
): boolean => { /* ... */ };

Then import in both XMarkdown.ts and XMarkdownAttributes.ts.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/web-platform/web-elements/src/elements/XMarkdown/XMarkdown.ts`
around lines 16 - 91, The three duplicated helpers (getComposedRange,
createRangeByOffsets, isSelectionNodeInsideHost) should be extracted into a
single shared module and imported where needed: create a new
XMarkdownSelectionUtils.ts exporting those three functions (keeping their
current signatures and behavior), replace the local definitions in XMarkdown.ts
and XMarkdownAttributes.ts with imports from XMarkdownSelectionUtils, and update
any references to use the imported symbols so both files use the same
implementation.

packages/web-platform/web-elements/src/elements/XMarkdown/XMarkdownAttributes.ts (2)

1028-1054: Typewriter timer interval recalculates on each restart but animation step persists.

When #startTypewriterTimer() is called (e.g., from velocity change on line 1147), it stops and restarts the timer with a new interval. However, if #animationStep has already reached #maxAnimationStep, the interval callback immediately stops again on line 1051. This is correct behavior, but the re-creation of the interval is wasteful. Consider guarding against restart when animation is already complete.

♻️ Suggested improvement

   `@registerAttributeHandler`('animation-velocity', true)
   _handleAnimationVelocity(newVal: string | null) {
     const n = Number(newVal);
     if (!Number.isNaN(n) && n > 0) this.#animationVelocity = n;
-    if (this.#animationType === 'typewriter' && this.#animationTimer) {
+    if (
+      this.#animationType === 'typewriter'
+      && this.#animationTimer
+      && this.#animationStep < this.#maxAnimationStep
+    ) {
       this.#startTypewriterTimer();
     }
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/web-platform/web-elements/src/elements/XMarkdown/XMarkdownAttributes.ts`
around lines 1028 - 1054, The timer restart in `#startTypewriterTimer`() should be
avoided when the animation is already complete; add an early guard at the top of
`#startTypewriterTimer`() that returns immediately if this.#animationStep >=
this.#maxAnimationStep (before calling `#stopTypewriterTimer` or creating a new
setInterval), so you don't recreate an interval only to stop it immediately;
keep the rest of the function (interval calculation, dispatch,
`#renderTypewriter`, and `#stopTypewriterTimer` usage) unchanged.

---

543-577: Event listeners should be scoped to the component's document.

The selection event listeners are attached to document (lines 571-574), but if this component is used within an iframe or different document context, using this.#dom.ownerDocument would be more robust.

♻️ Suggested improvement

   `#ensureEvents`() {
     if (this.#eventsAttached) return;
     const root = this.#root();
+    const doc = this.#dom.ownerDocument;
     root.addEventListener('click', this.#handleClick);
     this.#eventsAttached = true;
     if (!this.#selectionEventAttached) {
-      document.addEventListener('selectionchange', this.#handleSelectionChange);
-      document.addEventListener('mouseup', this.#handleSelectionGestureEnd);
-      document.addEventListener('touchend', this.#handleSelectionGestureEnd);
-      document.addEventListener('keyup', this.#handleSelectionGestureEnd);
+      doc.addEventListener('selectionchange', this.#handleSelectionChange);
+      doc.addEventListener('mouseup', this.#handleSelectionGestureEnd);
+      doc.addEventListener('touchend', this.#handleSelectionGestureEnd);
+      doc.addEventListener('keyup', this.#handleSelectionGestureEnd);
       this.#selectionEventAttached = true;
     }
   }

Apply the same change in dispose().

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/web-platform/web-elements/src/elements/XMarkdown/XMarkdownAttributes.ts`
around lines 543 - 577, The selection-related listeners are being added/removed
on the global document which breaks when the component lives in another document
(e.g. an iframe); update `#ensureEvents` and dispose to use the component's owner
document (use this.#dom.ownerDocument or the document obtained via the same
logic used by `#root`()) when calling addEventListener/removeEventListener for
`#handleSelectionChange` and `#handleSelectionGestureEnd`, ensuring you use the same
ownerDocument reference in both attachment and cleanup and keep the existing
flags (`#selectionEventAttached`) and timeout clearing logic unchanged.

packages/web-platform/web-elements/src/elements/XMarkdown/XMarkdownAttributes.ts (1)

565-576: Consider lazy attachment of selection event listeners.

Selection-related document event listeners (selectionchange, mouseup, touchend, keyup) are attached unconditionally in #ensureEvents(), even when text-selection is disabled. While the handlers early-return when #textSelection is false, this adds unnecessary overhead.

Consider attaching these listeners only when text-selection becomes truthy and removing them when it becomes falsy in _handleTextSelection.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/web-platform/web-elements/src/elements/XMarkdown/XMarkdownAttributes.ts`
around lines 565 - 576, The selection-related document listeners are being
attached unconditionally in `#ensureEvents`; change that so `#ensureEvents` only
attaches the local click listener (root.addEventListener('click',
this.#handleClick)) and does not attach selection listeners, then implement lazy
attach/detach of selection listeners inside _handleTextSelection: when
this.#textSelection becomes truthy, add document listeners for 'selectionchange'
-> this.#handleSelectionChange and for 'mouseup', 'touchend', 'keyup' ->
this.#handleSelectionGestureEnd and set this.#selectionEventAttached = true;
when this.#textSelection becomes falsy, remove those same document listeners and
set this.#selectionEventAttached = false. Keep using the existing handler names
(`#handleSelectionChange`, `#handleSelectionGestureEnd`) and the flag
(`#selectionEventAttached`) to avoid duplicate attachments.

.changeset/nasty-lizards-refuse.md (1)

7-7: ⚠️ Potential issue | 🟡 Minor

Fix the component name typo.

x-markdonw should be x-markdown in the release note and import guidance.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.changeset/nasty-lizards-refuse.md at line 7, Update the release note and
import guidance to correct the component name typo: replace all occurrences of
the incorrect identifier "x-markdonw" with the correct component name
"x-markdown" so imports and documentation reference the proper opt-in importing
pattern and component usage.

packages/web-platform/web-elements/index.css (1)

16-16: ⚠️ Potential issue | 🟡 Minor

Use the repo's quoted import notation.

The added import still trips the import-notation Stylelint rule on this PR. Switch just this line to the quoted form to keep the change lint-clean.

Suggested fix

-@import url("./src/elements/XMarkdown/x-markdown.css");
+@import "./src/elements/XMarkdown/x-markdown.css";

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/web-platform/web-elements/index.css` at line 16, The `@import` line
using url(...) triggers the import-notation Stylelint rule; replace the
statement `@import` url("./src/elements/XMarkdown/x-markdown.css"); in index.css
with the repo's quoted import notation by using a plain quoted import string for
"./src/elements/XMarkdown/x-markdown.css" (i.e., `@import`
"./src/elements/XMarkdown/x-markdown.css";) so the import-notation rule no
longer flags it and the change is lint-clean.

packages/web-platform/web-elements/src/elements/XMarkdown/XMarkdownAttributes.ts (3)

1255-1259: ⚠️ Potential issue | 🟠 Major

Don't move the source truncation/cursor node out of light DOM.

Both paths appendChild() the original node into the shadow tree. After the marker/cursor is removed or the component rerenders, that source node is gone, so the same truncation view or custom cursor can't be reused on later updates. Append a deep clone or slot the source node instead.

Also applies to: 1300-1305

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/web-platform/web-elements/src/elements/XMarkdown/XMarkdownAttributes.ts`
around lines 1255 - 1259, The code moves the original truncation/cursor node
from light DOM into the shadow by using span.appendChild(view), which removes it
from its source and prevents reuse; instead, when (cfg as any).truncationType
=== 'view' and a view node is found via this.#dom.querySelector(String((cfg as
any).content)), append a deep clone (e.g., view.cloneNode(true)) or slot the
original into the shadow rather than calling appendChild(view) so the source
node remains in light DOM and can be reused across rerenders; update the logic
around span.appendChild(view) (and the analogous block around lines 1300-1305)
to use cloning or a slot mechanism and ensure any event listeners or references
are correctly handled for the cloned element.

---

661-668: ⚠️ Potential issue | 🟠 Major

Reset typewriter state when content becomes empty.

This branch clears the DOM, but it leaves any active timer/cursor/step state behind. Clearing content mid-animation can keep the old interval alive and leak stale cursor state into the next render.

Possible fix

   if (!this.#content) {
+    this.#stopTypewriterTimer();
+    this.#removeTypewriterCursor();
+    this.#animationStarted = false;
+    this.#animationStep = 0;
+    this.#maxAnimationStep = 0;
     root.innerHTML = '';
     this.#renderedContent = '';
     this.#appendRemainder = '';

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/web-platform/web-elements/src/elements/XMarkdown/XMarkdownAttributes.ts`
around lines 661 - 668, When content is cleared in `#render`(), you must also
reset the typewriter animation state so timers and cursor/step state don't
persist; in addition to clearing DOM and setting `#renderedContent` and
`#appendRemainder`, call the internal reset/stop logic (e.g. invoke any existing
method like `#clearAppendFlushTimer` and also stop/clear the typewriter timer,
reset cursor visibility and step counters — e.g. `#stopTypewriter`(), clear
`#appendFlushTimer`, set cursor/visibility flags and step/index fields to their
initial values) so no interval or cursor state leaks into the next render.

---

1111-1178: ⚠️ Potential issue | 🟠 Major

These runtime attribute updates are still latent until another render.

markdown-style, content-complete, typewriter-dynamic-height, typewriter-height-transition-duration, and text-maxline all feed cursor/truncation/post-render behavior, but these handlers only update cached state or the style tag. Updating them after first paint leaves the DOM in the old state until some unrelated content or animation tick reruns #applyPostRenderPolicies().

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/web-platform/web-elements/src/elements/XMarkdown/XMarkdownAttributes.ts`
around lines 1111 - 1178, Handlers _handleMarkdownStyle, _handleContentComplete,
_handleTypewriterDynamicHeight, _handleTypewriterHeightTransition, and
_handleTextMaxline only update cached state or style but do not trigger the
post-render fixes, leaving the DOM stale; after updating their internal state
and calling `#updateStyleTag`() (where present), call the component's post-render
routine (this.#applyPostRenderPolicies()) and/or schedule a render
(this.#scheduleRender()) so cursor/truncation/height behaviors are recomputed
immediately; modify each handler to invoke these methods (use
`#applyPostRenderPolicies`() first, then `#scheduleRender`() if needed) to force
reapplication of post-render policies when those attributes change at runtime.

packages/web-platform/web-elements/src/elements/XMarkdown/XMarkdown.ts (1)

193-205: ⚠️ Potential issue | 🟠 Major

pauseAnimation() / resumeAnimation() still don't control a live typewriter.

pauseAnimation() only writes animation-velocity="0", but XMarkdownAttributes._handleAnimationVelocity() ignores non-positive values and leaves the running interval alive. resumeAnimation() then rewrites animation-type="typewriter", which is ineffective when the element never left that mode, so this public API still has no reliable pause/resume path.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/web-platform/web-elements/src/elements/XMarkdown/XMarkdown.ts`
around lines 193 - 205, The pause/resume public API doesn't actually stop the
running typewriter because pauseAnimation only sets animation-velocity="0" while
XMarkdownAttributes._handleAnimationVelocity ignores non-positive values; update
pauseAnimation to actively stop the live typewriter by calling the internal
stop/cleanup routine (or clearing the interval/timer) used by
XMarkdown/XMarkdownAttributes (refer to the typewriter interval/stop logic and
any method like _handleAnimationVelocity, stopTypewriter,
clearTypewriterInterval), and update resumeAnimation to set a positive
animation-velocity (e.g., "40" or the provided initial-animation-step logic) and
then call the same internal start handler (or _handleAnimationVelocity) to
restart the interval; ensure resumeAnimation does not rely solely on toggling
animation-type and that both methods invoke the same internal start/stop code
paths so pause/resume is reliable.

packages/web-platform/web-elements/tests/x-markdown.spec.ts (1)

349-350: Replace fixed sleeps with observable waits.

These assertions are coupled to the current timer values (50ms, 20/80ms, 500ms, 1500ms, 1000ms). They will get flaky as soon as CI load or animation timing changes. Prefer waitForFunction(...) or a Playwright assertion tied to the DOM state/event you actually need. Based on learnings, tests should use standard Playwright assertions like expect(locator)... for UI-state checks.

Also applies to: 437-440, 531-540, 552-552

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/web-platform/web-elements/tests/x-markdown.spec.ts` around lines 349
- 350, The test uses fixed sleeps (e.g., page.waitForTimeout(50)) after helper
selectShadowText which makes tests flaky; replace these waits with observable
waits that assert the actual DOM/selection state instead. Locate calls to
selectShadowText and the subsequent page.waitForTimeout (including the other
occurrences referenced) and change them to await page.waitForFunction(...) or
await expect(<appropriate Locator>).toHaveText/toHaveClass/visibility checks
tied to the exact DOM mutation or selection change you need (or use
locator.waitFor when waiting for an element state). Ensure the wait checks the
concrete condition produced by selectShadowText (selection range text, attribute
change, or element content) rather than sleeping for a fixed duration.

packages/web-platform/web-elements/src/elements/XMarkdown/XMarkdown.ts (1)

174-177: Use a cached genDomGetter for #markdown-root lookups.

This file repeatedly re-queries the same shadow node with ad-hoc null checks. A single genDomGetter keeps the access pattern consistent with the rest of the package and avoids repeating the same unsafe lookup logic. As per coding guidelines, "Use genDomGetter to safely access internal elements within the Shadow DOM".

Also applies to: 208-223, 301-307

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/web-platform/web-elements/src/elements/XMarkdown/XMarkdown.ts`
around lines 174 - 177, Multiple methods (including getImages) re-query
shadowRoot for '#markdown-root' with ad-hoc null checks; replace these with a
single cached genDomGetter instance to centralize safe Shadow DOM access. Add a
private cached getter (e.g., this._getMarkdownRoot =
genDomGetter('#markdown-root')) on the component and refactor getImages and the
other methods that query '#markdown-root' (the blocks around lines 208-223 and
301-307) to call that getter, handle null/HTMLElement typing consistently, and
remove the repeated shadowRoot?.querySelector logic.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

packages/web-platform/web-elements/src/elements/XMarkdown/XMarkdownAttributes.ts (4)

1119-1140: ⚠️ Potential issue | 🟠 Major

These attribute updates stay stale until another render happens.

The handlers update internal state or the style tag, but truncation markers, cursor behavior, overflow emission, and dynamic-height transition are only recalculated inside #applyPostRenderPolicies(). Toggling these attributes after first paint leaves the DOM in the old state until some unrelated rerender occurs.

Also applies to: 1165-1186

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/web-platform/web-elements/src/elements/XMarkdown/XMarkdownAttributes.ts`
around lines 1119 - 1140, The attribute handlers (_handleMarkdownStyle,
_handleContentId, _handleTextSelection) currently only update internal state or
style tag so UI behaviors (truncation markers, cursor, overflow emission,
dynamic-height transitions) remain stale; call or schedule a post-render
recalculation by invoking the existing `#applyPostRenderPolicies`() after updating
state (or, if timing matters, queue it with requestAnimationFrame/microtask) and
keep the existing `#updateStyleTag`() call in _handleTextSelection; ensure
`#applyMarkdownStyle`, `#contentId` and `#textSelection` assignments are done before
calling `#applyPostRenderPolicies`() so the recalculation uses the new values.

---

674-681: ⚠️ Potential issue | 🟠 Major

Reset the typewriter session when content changes or clears.

These paths keep the previous timer/cursor/progress alive. Clearing content can leave the old animation running, and replacing content in typewriter mode reuses the old #animationStep, so the next payload can start mid-stream or finish immediately.

Also applies to: 1098-1106

---

1263-1268: ⚠️ Potential issue | 🟠 Major

Clone light-DOM views before injecting them into truncation/cursor containers.

Both branches appendChild(view) the original node. After the next rerender or cursor removal, that source view is no longer in light DOM, so the same custom truncation view/cursor cannot be injected again.

Also applies to: 1308-1314

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/web-platform/web-elements/src/elements/XMarkdown/XMarkdownAttributes.ts`
around lines 1263 - 1268, The truncation/cursor injection currently does
span.appendChild(view) which moves the original light-DOM node out of source,
preventing reuse; change both places that append the found view (the branch
where (cfg as any).truncationType === 'view' and the similar cursor-injection
block) to append a cloned node instead by using view.cloneNode(true) (deep
clone) before appending to span so the original light-DOM view remains intact
for future renders.

---

1150-1157: ⚠️ Potential issue | 🟠 Major

Treat non-positive animation-velocity as an actual pause/stop.

XMarkdown.pauseAnimation() writes animation-velocity="0", but Line 1153 ignores 0 and leaves the running interval untouched. That makes the public pause API ineffective while the typewriter is active.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/web-platform/web-elements/src/elements/XMarkdown/XMarkdownAttributes.ts`
around lines 1150 - 1157, _handleAnimationVelocity currently ignores zero so
XMarkdown.pauseAnimation (which writes animation-velocity="0") doesn't stop the
typewriter; change the handler to parse newVal into n, if Number.isNaN(n)
return, otherwise assign this.#animationVelocity = n; then if
this.#animationType === 'typewriter' and this.#animationTimer: if n > 0 call
this.#startTypewriterTimer() else stop the timer (clear the interval/timeout and
set this.#animationTimer = null) so a non-positive velocity properly
pauses/stops the animation.

packages/web-platform/web-elements/src/elements/XMarkdown/XMarkdown.ts (2)

251-304: ⚠️ Potential issue | 🟠 Major

Honor indexType: 'source' or remove it from the API.

Line 252 advertises source-based indexing, but Lines 294-299 always build the range against rendered text nodes. As soon as the markdown contains syntax characters, "source" offsets point at the wrong substring and the returned rect is wrong.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/web-platform/web-elements/src/elements/XMarkdown/XMarkdown.ts`
around lines 251 - 304, getTextBoundingRect advertises an indexType:'source'
option but the current implementation always treats start/end as rendered
character offsets; either implement source→rendered mapping or remove the
option. Fix by updating getTextBoundingRect (and the helper createRangeByChar)
to check params.indexType: if 'source' convert the provided source offsets to
rendered-text offsets (e.g., walk the original markdown/source-to-render mapping
and translate start/end into the positions used by createRangeByChar) before
calling createRangeByChar; if you don't want to support 'source' remove
indexType from the signature and any callers. Ensure you reference
getTextBoundingRect, params.indexType, and createRangeByChar when making the
change so the correct conversion or API removal is applied.

---

335-375: ⚠️ Potential issue | 🟠 Major

getParseResult() truncates ranges for nested markup.

calcOffset() stops on the first direct text-node hit, so tags whose rendered text spans multiple descendant nodes only report the first fragment. A node like <p>Hello <strong>world</strong></p> will under-report end.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/web-platform/web-elements/src/elements/XMarkdown/XMarkdown.ts`
around lines 335 - 375, getParseResult's calcOffset only captures the first text
fragment because it breaks as soon as it finds a text node that is the target or
its direct child; change calcOffset (used in getParseResult) to scan all text
nodes under root and record the position of the first text node that is inside
the target node and the position at the end of the last text node that is inside
the target (use node.nodeType === Node.TEXT_NODE to detect text nodes and
node.contains(current) to test descendant membership). In short: when iterating
the TreeWalker, don't break on the first match—set start on the first matching
text node and keep updating end for each subsequent matching text node so
elements like <p>Hello <strong>world</strong></p> return a start at "Hello" and
end after "world".

packages/web-platform/web-elements/tests/x-markdown.spec.ts (1)

185-209: Keep the event test hermetic.

This case clicks a live https://example.com link. If the component ever stops preventing default, the test starts depending on external navigation instead of just validating bindlink/bindimageTap. Stub or route that URL before clicking.

As per coding guidelines: "Mock external requests in tests using page.route to ensure tests are hermetic and fast".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/web-platform/web-elements/tests/x-markdown.spec.ts` around lines 185
- 209, The test clicks a real external link which can cause navigation; before
the click in the 'should fire bindlink and bindimageTap events' test, register a
Playwright route (using page.route) for the target URL (e.g.,
'https://example.com/*') to intercept and fulfill a synthetic response (200 +
minimal body) so the click stays in-process; make the page.route call prior to
calling markdown.locator('a').click() and remove or dispose the route after the
assertions to keep the test hermetic and ensure the bindlink/bindimageTap
assertions run reliably.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes